-
Notifications
You must be signed in to change notification settings - Fork 1
Redundant class cleanup #86
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
UnitType=Units.CURRENCY, | ||
PreferredUnits=CurrencyUnit.MDOLLARS, | ||
CurrentUnits=CurrencyUnit.MDOLLARS, | ||
) | ||
self.Coam = self.OutputParameterDict[self.Coam.Name] = OutputParameter( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parent class units are MDOLLARSPERYEAR
while SUTRA units are KDOLLARSPERYEAR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same applies for self.annualpumpingcosts
on line 44 below, which is not commentable in this PR due to technically not being part of the diff.
PreferredUnits=CurrencyUnit.MDOLLARS, | ||
CurrentUnits=CurrencyUnit.MDOLLARS, | ||
) | ||
|
||
self.annualngcost = self.OutputParameterDict[self.annualngcost.Name] = OutputParameter( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parent class units are MDOLLARSPERYEAR
while SUTRA units are KDOLLARSPERYEAR
@@ -347,7 +126,7 @@ def Calculate(self, model: Model) -> None: | |||
# Drilling | |||
|
|||
self.C1well = 0 | |||
if self.ccwellfixed.Valid: | |||
if self.per_production_well_cost.Valid: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that Economics.ccwellfixed
was renamed to per_production_well_cost
in ac08741#diff-372dea2554a92733824c98709c9bdb5720c183c31762c3795bd3990f069a2f24R600; thus this change is simply to use the correct variable instead of redefining it redundantly with the old name.
self.C1well = self.C1well * self.ccwelladjfactor.value | ||
self.C1well = self.C1well * self.production_well_cost_adjustment_factor.value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that ccwelladjfactor
was refactored to injection_well_cost_adjustment_factor
in NREL@ac08741 as well
Description
SBTEconomics
andSUTRAEconomics
that already exists inEconomics
parent classSUTRAEconomics
because it was incidentally trivial to implement in the course of refactoring. (Hardcoded peaking boiler cost in SUTRA was overlooked byPeaking Boiler Cost per kW
; Increase allowed fractures; HonorCapital Cost for Power Plant
; Client caching fix;Cape-4
laterals & res. vol.; Doublets parameter [v3.9.28] NREL/GEOPHIRES-X#394, which resolved Feature Request: DH LCOH with peaking boiler cost = 0 NREL/GEOPHIRES-X#392.)Testing & Verification
SUTRAEconomics
retains a few seemingly-redundant parameters from parent class (Economics
) includingannualngcost
,LCOH
andCoam
. However, they are not actually redundant because use different units than the parent class, and thus removing their re-declarations inSUTRAEconomics
would be a backwards-incompatible change.